Skip to content

Add SpecializedFunction. #7718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 21, 2025
Merged

Add SpecializedFunction. #7718

merged 2 commits into from
May 21, 2025

Conversation

ilyalesokhin-starkware
Copy link
Contributor

No description provided.

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft April 29, 2025 10:06
@reviewable-StarkWare
Copy link

This change is Reviewable

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/specialized_func branch 2 times, most recently from 701a776 to c71e53c Compare April 29, 2025 15:36
Copy link
Contributor Author

ilyalesokhin-starkware commented Apr 29, 2025

@ilyalesokhin-starkware
Copy link
Contributor Author

crates/cairo-lang-lowering/src/lower/test_data/specialized line 24 at r3 (raw file):


//! > full_path
test::foo{None, 1, }

do you have an idea for a format that is parsable by lalrpop?

Code quote:

test::foo{None, 1, }

@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from main to graphite-base/7718 May 14, 2025 07:48
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/7718 to ilya/external_size May 14, 2025 07:48
Base automatically changed from ilya/external_size to main May 14, 2025 08:48
@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review May 14, 2025 08:49
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 8 of 57 files at r5, 51 of 51 files at r6, all commit messages.
Reviewable status: 57 of 58 files reviewed, 8 unresolved discussions


crates/cairo-lang-lowering/src/inline/mod.rs line 40 at r6 (raw file):

        FunctionWithBodyLongId::Generated { .. } => InlineConfiguration::None,
        FunctionWithBodyLongId::Specialized(specialized) => db.function_declaration_inline_config(
            specialized.base.base_semantic_function(db).function_with_body_id(db),

what if this is the specialization of a generated function?

Code quote:

            specialized.base.base_semantic_function(db).function_with_body_id(db),

crates/cairo-lang-lowering/src/inline/mod.rs line 261 at r6 (raw file):

                {
                    if specialized.base == called_func {
                        // a specialized function should always inline it base.

Suggestion:

                        // A specialized function should always inline its base.

crates/cairo-lang-lowering/src/lower/specialized_test.rs line 49 at r6 (raw file):

        base: function_id,
        args: Arc::new([None, Some(ConstValue::Int(BigInt::one(), core.felt252))]),
    };

further doc the test - this is no longer the standard lowering test.

Code quote:

    let specialized_func = SpecializedFunction {
        base: function_id,
        args: Arc::new([None, Some(ConstValue::Int(BigInt::one(), core.felt252))]),
    };

crates/cairo-lang-lowering/src/ids.rs line 330 at r6 (raw file):

                        None
                    })
                    .collect::<Vec<_>>();

Suggestion:

                base_sign.params = zip_eq(base_sign.params, &specialized.args)
                    .filter_map(|(param, arg)| arg.is_none().then_some(param))
                    .collect();

crates/cairo-lang-lowering/src/ids.rs line 473 at r6 (raw file):

/// Specialized function.
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct SpecializedFunction {

further doc what Specialized means in this case.


crates/cairo-lang-lowering/src/concretize/mod.rs line 27 at r6 (raw file):

            .intern(db))
        }
        FunctionLongId::Specialized(_) => unreachable!("This should not be called."),

Suggestion:

        FunctionLongId::Specialized(_) => unreachable!("Specialization of functions only occurs post concretization."),

crates/cairo-lang-lowering/src/db.rs line 509 at r6 (raw file):

                    parameters,
                    diagnostics: Default::default(),
                }

extract to function - this seems no longer trivial.

Code quote:

            {
                let base = db.lowered_body(specialized.base, LoweringStage::Monomorphized)?;

                let base_semantic = specialized.base.base_semantic_function(db);

                let mut variables = VariableAllocator::new(
                    db,
                    base_semantic.function_with_body_id(db),
                    Default::default(),
                )?;
                let mut statement = vec![];
                let mut parameters = vec![];
                for (param, arg) in zip_eq(&base.parameters, specialized.args.iter()) {
                    let var_id = variables.variables.alloc(base.variables[*param].clone());
                    if let Some(arg) = arg {
                        statement.push(Statement::Const(StatementConst {
                            value: arg.clone(),
                            output: var_id,
                        }));
                        continue;
                    }
                    parameters.push(var_id);
                }

                let location = LocationId::from_stable_location(
                    db,
                    specialized.base.base_semantic_function(db).stable_location(db),
                );
                let inputs = variables
                    .variables
                    .iter()
                    .map(|(var_id, _)| VarUsage { var_id, location })
                    .collect();

                let outputs: Vec<VariableId> = chain!(
                    base.signature.extra_rets.iter().map(|ret| ret.ty()),
                    [base.signature.return_type]
                )
                .map(|ty| variables.new_var(VarRequest { ty, location }))
                .collect_vec();

                let mut block_builder = BlocksBuilder::new();

                let ret_usage = outputs
                    .iter()
                    .map(|var_id| VarUsage { var_id: *var_id, location })
                    .collect_vec();

                statement.push(Statement::Call(StatementCall {
                    function: specialized.base.function_id(db)?,
                    with_coupon: false,
                    inputs,
                    outputs,
                    location,
                }));
                block_builder.alloc(Block {
                    statements: statement,
                    end: BlockEnd::Return(ret_usage, location),
                });
                Lowered {
                    signature: function.signature(db)?,
                    variables: variables.variables,
                    blocks: block_builder.build().unwrap(),
                    parameters,
                    diagnostics: Default::default(),
                }

Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 57 of 58 files reviewed, 8 unresolved discussions (waiting on @orizi)


crates/cairo-lang-lowering/src/ids.rs line 473 at r6 (raw file):

Previously, orizi wrote…

further doc what Specialized means in this case.

Done.


crates/cairo-lang-lowering/src/inline/mod.rs line 40 at r6 (raw file):

Previously, orizi wrote…

what if this is the specialization of a generated function?

Done.


crates/cairo-lang-lowering/src/lower/specialized_test.rs line 49 at r6 (raw file):

Previously, orizi wrote…

further doc the test - this is no longer the standard lowering test.

Done.


crates/cairo-lang-lowering/src/ids.rs line 330 at r6 (raw file):

                        None
                    })
                    .collect::<Vec<_>>();

Done.


crates/cairo-lang-lowering/src/concretize/mod.rs line 27 at r6 (raw file):

            .intern(db))
        }
        FunctionLongId::Specialized(_) => unreachable!("This should not be called."),

Done.


crates/cairo-lang-lowering/src/inline/mod.rs line 261 at r6 (raw file):

                {
                    if specialized.base == called_func {
                        // a specialized function should always inline it base.

Done.

Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 57 of 58 files reviewed, 8 unresolved discussions (waiting on @orizi)


crates/cairo-lang-lowering/src/db.rs line 509 at r6 (raw file):

Previously, orizi wrote…

extract to function - this seems no longer trivial.

Done.

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)


crates/cairo-lang-lowering/src/lower/test_data/specialized line 24 at r3 (raw file):

Previously, ilyalesokhin-starkware wrote…

do you have an idea for a format that is parsable by lalrpop?

https://reviewable.io/reviews/starkware-libs/cairo/7752

@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from main to graphite-base/7718 May 15, 2025 11:29
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/7718 to ilya/avoid_to_func_with_body May 15, 2025 11:30
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)

@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from ilya/avoid_to_func_with_body to graphite-base/7718 May 15, 2025 12:52
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/7718 to main May 15, 2025 12:52
@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/specialized_func branch 2 times, most recently from 6d772b3 to 9cac382 Compare May 15, 2025 13:22
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 3 files at r9, 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware)


crates/cairo-lang-lowering/src/ids.rs line 103 at r10 (raw file):

);

pub enum GenericOrSpecialized {

doc

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r11, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r12, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

@ilyalesokhin-starkware ilyalesokhin-starkware added this pull request to the merge queue May 21, 2025
Merged via the queue into main with commit e606b12 May 21, 2025
49 checks passed
@orizi orizi deleted the ilya/specialized_func branch May 23, 2025 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants